Skip to content

SIP338 Remove schema version from restart.c#341

Merged
Alomir merged 2 commits into
PecanProject:masterfrom
re2zero:sip338-remove-schema-version
Jun 1, 2026
Merged

SIP338 Remove schema version from restart.c#341
Alomir merged 2 commits into
PecanProject:masterfrom
re2zero:sip338-remove-schema-version

Conversation

@re2zero

@re2zero re2zero commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Remove RESTART_SCHEMA_VERSION and related validation code. Model version is sufficient for version checking.

Changes:

  • Remove RESTART_SCHEMA_VERSION constant
  • Remove schema version validation in readRestartState
  • Update header format from 'SIPNET_RESTART 1.0' to 'SIPNET_RESTART'
  • Remove testSchemaMismatchFails test
  • Update documentation

How was this change tested?

  • All existing unit tests pass
  • testRestartMVP passes with the schema version test removed

Related issues

Checklist

  • Related issues are listed above.
  • PR title has the issue number in it ("SIP338 <concise description of proposed change>")
  • Tests added/updated for new features (if applicable)
  • Documentation updated
  • docs/CHANGELOG.md updated with noteworthy changes
  • Code formatted with clang-format (run git clang-format if needed)

Remove RESTART_SCHEMA_VERSION and related validation code.
Model version is sufficient for version checking.

- Remove RESTART_SCHEMA_VERSION constant
- Remove schema version validation in readRestartState
- Update header format from 'SIPNET_RESTART 1.0' to 'SIPNET_RESTART'
- Remove testSchemaMismatchFails test
- Update documentation

Fixes PecanProject#338

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the dedicated restart “schema version” from SIPNET restart checkpoints, relying on the model numeric version and existing schema-layout guards for compatibility checking.

Changes:

  • Simplify restart checkpoint header from SIPNET_RESTART 1.0 to SIPNET_RESTART and remove schema-version validation.
  • Update restart MVP tests to reflect the new header and drop the schema-version mismatch test.
  • Update restart checkpoint documentation and note the change in the Unreleased changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/sipnet/restart.c Removes RESTART_SCHEMA_VERSION, drops schema-version parsing/validation, updates header write format.
tests/sipnet/test_restart_infrastructure/testRestartMVP.c Updates expected header and removes schema-version mismatch test coverage.
docs/developer-guide/restart-checkpoint.md Updates restart checkpoint spec/docs to remove schema version references and reflect new header.
docs/CHANGELOG.md Records the restart checkpoint format change under Unreleased “Removed”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/sipnet/restart.c Outdated
Comment thread docs/developer-guide/restart-checkpoint.md

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@re2zero Thank you for the contribution! This looks great, only two small tweaks: copillot's comment above, and make sure to use 'clang-format' to get the cpp-linter check ot pass.

- Replace sscanf-based header parsing with exact strcmp against
  RESTART_MAGIC, rejecting any trailing content after the magic string
- Run clang-format to fix formatting issues in _Static_assert and
  logError lines, resolving cpp-linter check failure

Signed-off-by: re2zero <yangwu@uniontech.com>
Comment thread src/sipnet/restart.c
Comment on lines +542 to +543
// Strip trailing newline/carriage return for exact comparison
firstLine[strcspn(firstLine, "\r\n")] = '\0';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

@Alomir Alomir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@Alomir Alomir merged commit 59daaa3 into PecanProject:master Jun 1, 2026
12 checks passed
@re2zero re2zero deleted the sip338-remove-schema-version branch June 2, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove schema version from restart.c

3 participants